-
-
Notifications
You must be signed in to change notification settings - Fork 405
West Midlands | Jan-2026 | Arunkumar Akilan | Sprint 1 | Wireframe to Web Code #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
West Midlands | Jan-2026 | Arunkumar Akilan | Sprint 1 | Wireframe to Web Code #995
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The content in the PR Description is not yet properly formatted using the Markdown syntax. Can you update its format accordingly? To learn the Markdown syntax, please refer to |
cjyuan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
When a wireframe is provided, our implementation should closely reflect its appearance and layout to ensure consistency with design expectations. You're off to a solid start. To better align with the wireframe, can you center the PAGE TITLE and the PAGE SUBTITLE (the part that replace "A SHORT DESCRIPTION")?
-
(Optional change) If you are up to some challenge to further improve the layout, leave more space above and below the border of READ ME link, and keep the spacing consistent regardless of the amount of content in the articles.
cjyuan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me.
When it comes to CSS, I am afraid AI can probably help you more than I do. Have you tried asking AI about possible improvements on your CSS code?
| max-width: var(--container); | ||
| margin: 0 auto calc(var(--space) * 4) auto; | ||
| padding-bottom: 80px; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two main selectors (lines 57 and 95). It is better to merge them into one. (I know you didn't write them, but that's one possible improvement you can easily do).
There are also a few selectors that are related to main (lines 121-128). It is better to place all the selectors related to main element together.

Self checklist
Changelist
Questions